-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Key render phases off the main world view entity, not the render world view entity. #16942
Conversation
view entity. We won't be able to retain render phases from frame to frame if the keys are unstable. It's not as simple as simply keying off the main world entity, however, because some main world entities extract to multiple render world entities. For example, directional lights extract to multiple shadow cascades, and point lights extract to one view per cubemap face. Therefore, we key off a new type, `RetainedViewEntity`, which contains the main entity plus a *subview ID*. This is part of the preparation for retained bins.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing gets drawn for the UI, but I'm not very familiar with the camera extraction internals and I couldn't quite unravel the problem.
Very loosely, from what I can tell:
extract_default_ui_camera_view
spawns a default camera view entity for each camera which has an ExtractedView
with subview_index: 1
and then creates a corresponding TransparentUi
render phase.
But the extracted uinodes are given the entity id of the non-ui camera ExtractedView
entity so that in queue_uinodes
the non-ui view with subview_index: 0
is retrieved and then no TransparentUi
phases are found because they were all keyed to subview_index: 1
.
Setting the subview index to 1 in extract_cameras
fixes the UI examples but that breaks everything else of course.
This should fix the issue with the UI. The proper fix was a bit tricky and involved creating a new subgraph runner to make sure to run the UI rendering subgraph on the right render world entity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multi-window UI isn't working, I think I found the problem though.
Otherwise everything looks good now, the different shaders etc all seem to be working correctly.
Shadows and texture slicing isn't working either.
It looks like your PR is a breaking change, but you didn't provide a migration guide. Could you add some context on what users should update when this change get released in a new version of Bevy? |
It looks like there's still some issues with UI based on the example runner https://pixel-eagle.com/project/B25A040A-A980-4602-B90C-D480AB84076D?filter=PR-16942 |
Ah yeah you're right. Shadows and texture slices still don't work, I must have ran those examples on the wrong branch by mistake. |
The extraction and queue functions just needed to be updated. I put in a pr on this pr with the needed changes. |
…ing plugins to use the correct view entities.
Cherry picked over, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The UI rendering is all fixed now. There is a slight performance regression from the extra indirection but I don't think it's a problem with this PR really, it's more of a sign that we need to rethink how the UI handles camera views.
Let me know when this has passed an example check run and merge conflicts are resolved please :) |
@alice-i-cecile I kicked off a run. https://pixel-eagle.com/project/B25A040A-A980-4602-B90C-D480AB84076D?filter=PR-16942 Looks like the failures are known intermittents, so we should be good to go. |
* The retained view key from bevyengine#16942 was insufficient to uniquely identify a shadow cascade when multiple cameras were present. In such cases, the stable ID for a shadow cascade is actually (light entity, camera entity, cascade index), not (light entity, cascade index) as the PR in bevyengine#16942 assumed. This caused failures in the `camera_sub_view` example. * Sorted phase items didn't push batch sets as they were supposed to. I updated `batch_and_prepare_sorted_render_phase` to do so. This fixes the examples with transparency. * Unbatchable binned entities didn't push batch sets as they were supposed to. This fixes the `morph_targets` example. * As the `GpuPreprocessNode` now runs per camera (a necessary change for occlusion culling), it should only run on views associated with the current camera (the camera itself plus the shadow maps). It was running again for every view, causing failures in tests with multiple views like `split_screen`. * 3D meshes need to be re-extracted if their assets change, so that the first vertex and first index in `MeshInputUniform` are updated. I added a system to do so. Note that this system is somewhat inefficient when meshes change; once `cold-specialization` lands it can be updated to use the asset change infrastructure in that patch to fix the issue. This fixes the `query_gltf_primitives` example. * `specialized_mesh_pipeline` wasn't allocating indirect work items. I changed the example to do so.
We won't be able to retain render phases from frame to frame if the keys are unstable. It's not as simple as simply keying off the main world entity, however, because some main world entities extract to multiple render world entities. For example, directional lights extract to multiple shadow cascades, and point lights extract to one view per cubemap face. Therefore, we key off a new type,
RetainedViewEntity
, which contains the main entity plus a subview ID.This is part of the preparation for retained bins.